Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Relax statement type declaration #3677

Merged

Conversation

greg0ire
Copy link
Member

@greg0ire greg0ire commented Sep 16, 2019

Q A
Type improvement
BC Break no
Fixed issues n/a

Summary

fetch() is part of the ResultStatement interface. Using Statement as a
type declaration needlessly constrains people to use this class only
with implementations of Statement.

See

public function fetch($fetchMode = null, $cursorOrientation = PDO::FETCH_ORI_NEXT, $cursorOffset = 0);

@greg0ire
Copy link
Member Author

greg0ire commented Sep 16, 2019

Should I also rename the class to ResultStatementIterator (in a BC way, like this: https://dev.to/greg0ire/how-to-deprecate-a-type-in-php-48cf, which will be a pain)?

fetch() is part of the ResultStatement interface. Using Statement as a
type declaration needlessly constrains people to use this class only
with implementations of Statement.
@greg0ire greg0ire force-pushed the relax_statement_iterator_constraint branch from 75538e1 to 442c684 Compare September 16, 2019 15:18
@greg0ire
Copy link
Member Author

I tried completing the tests with ArrayStatement, but they fail because they assert that fetchAll is not called when calling getIterator() which is not true for ArrayStatement.

@greg0ire greg0ire requested a review from a team September 17, 2019 09:36
@morozov
Copy link
Member

morozov commented Sep 18, 2019

Should I also rename the class to ResultStatementIterator […]?

I don't think it's necessary. The entire hierarchy Statement extends ResultStatement looks odd. There's no reason for one to extend the other. There should be a statement and a result. If/once it's done, the iterator can be renamed accordingly.

@greg0ire greg0ire requested a review from a team October 23, 2019 21:09
@morozov morozov merged commit 8be62b2 into doctrine:master Oct 24, 2019
@morozov
Copy link
Member

morozov commented Oct 24, 2019

Thank you @greg0ire!

@greg0ire greg0ire deleted the relax_statement_iterator_constraint branch October 24, 2019 06:39
@morozov morozov self-assigned this Nov 2, 2019
@morozov morozov added this to the 2.10.0 milestone Nov 2, 2019
rgrellmann added a commit to Rossmann-IT/dbal that referenced this pull request Apr 22, 2020
# Release v2.10.0

This is a minor release of Doctrine DBAL that aggregates over 70 fixes and improvements developed by 25 contributors over the last year.

This release focuses on internal code quality improvement and deprecating the functionality identified for removal in the next major release.

## Backwards Compatibility Breaks

This release introduces a minor BC break. Default column values are no longer handled as SQL expressions. They are converted to SQL literals (e.g. escaped). The previous behavior was not portable and was never by design.

Clients must now specify default values in their initial form, not in the form of an SQL literal (e.g. escaped).

**Before:**

```php
$column->setDefault('Foo\\\\Bar\\\\Baz');
```

**After:**

```php
$column->setDefault('Foo\\Bar\\Baz');
```

## Deprecations

- The usage of the `getDriver()`, `getDatabasePlatform()` and `getSchemaManager()` methods of the `ConnectionEventArgs` class has been deprecated.
- The usage of the `getDatabasePlatform()` method of the `SchemaColumnDefinitionEventArgs` class has been deprecated.
- The usage of the `getHost()`, `getPort()`, `getUsername()` and `getPassword()` methods of the `Connection` class has been deprecated.
- Passing multiple SQL statements as an array to `SchemaAlterTableAddColumnEventArgs::addSql()` and the same method in other `SchemaEventArgs`-based classes is deprecated.
- Calling `AbstractSchemaManager::tablesExist()` with a string argument is deprecated.
- Calling `OracleSchemaManager::createDatabase()` without an argument or by passing `NULL` is deprecated.
- Unused schema manager methods are deprecated.
   -  `AbstractSchemaManager::_getPortableFunctionsList()`,
   - `AbstractSchemaManager::_getPortableFunctionDefinition()`,
   - `OracleSchemaManager::_getPortableFunctionDefinition()`,
   - `SqliteSchemaManager::_getPortableTableIndexDefinition()`.
- The usage of `NULL` to indicate empty `$username` or `$password` when calling `Doctrine\DBAL\Driver::connect()` is deprecated.
- Method `Doctrine\DBAL\Platforms::_getAlterTableIndexForeignKeySQL()` has been deprecated as no longer used.
- Property `Doctrine\DBAL\Driver\OCI8\OCI8Statement::$_PARAM` has been deprecated as not used.
- Method `Doctrine\DBAL\Driver::getName()` is deprecated.
- The usage of user-provided `PDO` instance is deprecated.
- `Type::*` constants are deprecated.
- The `Doctrine\DBAL\Driver\SQLSrv\SQLSrvStatement::LAST_INSERT_ID_SQL` constant has been deprecated.
- The constants in `Doctrine\DBAL\SQLParserUtils` have been deprecated.
- The `Doctrine\DBAL\Logging\LoggerChain::addLogger` method has been deprecated.

Please see the details in the [UPGRADE.md](UPGRADE.md) documentation.

## New Features and Improvements

- [3674: Add missing MySQL 8.0 reserved keywords](doctrine#3674) thanks to @loilo
- [3512: Support for comments on table in all databases](doctrine#3512) thanks to @moufmouf
- [3418: Add column charset for MySql](doctrine#3418) thanks to @altertable

**MySQL-related changes:**

 - [3668: Quote collation on MySQL](doctrine#3668) thanks to @staudenmeir
 - [3374: Clean up `MySqlPlatform::getListTableIndexesSQL()` fields](doctrine#3374) thanks to @BenMorel
 - [3311: Ensuring correct `ALTER TABLE` statement for creation of an `AUTO INCREMENT` column as new `PRIMARY KEY`](doctrine#3311) thanks to @arnegroskurth

**Driver level changes:**

- [3677: Relax statement type declaration](doctrine#3677) thanks to @greg0ire
- [3521: Maintain platform parameter in connection params](doctrine#3521) thanks to @jwage and @Perf
- [3588: Add commit result bool](doctrine#3588) thanks to @otazniksk

**Schema management:**

- [2960: Handle default values as values, not SQL expressions](doctrine#2960) thanks to @morozov

**Types improvements:**

 - [3356: Extract constants for built-in types from Type to Types](doctrine#3356) thanks to @Majkl578
 - [3354: Extract type factory and registry from Type into TypeRegistry](doctrine#3354) thanks to @Majkl578

**Compatibility with Symfony 5:**

 - [3706: add missing exit codes to ensure Symfony 5 compatibility](doctrine#3706) thanks to @dmaicher
 - [3563: Allow Symfony 5](doctrine#3563) thanks to @nicolas-grekas

**Query Builder:**

 - [3696: Add support for `DISTINCT` clause](doctrine#3696) thanks to @bingo-soft

**Logging:**

 - [3572: Make LoggerChain use constructor to add loggers instead of adder method](doctrine#3572) thanks to @ostrolucky

**Code quality improvements:**

 - [3667: Phpstan fix backport](doctrine#3667) thanks to @morozov
 - [3663: Updated PHPStan to v0.11.15](doctrine#3663) thanks to @morozov
 - [3604: Updated Jetbrains PhpStorm stubs to 2019.1](doctrine#3604) thanks to @morozov
 - [3549: Removed the assertion which doesn't work with a user-provided PDO connection](doctrine#3549) thanks to @morozov
 - [3489: Update doctrine coding standard from 5.0 to 6.0](doctrine#3489) thanks to @amaabdou
 - [3481: Updated PHPStan to v0.11.3](doctrine#3481) thanks to @morozov
 - [3443: PHPStan Level 7](doctrine#3443) thanks to @morozov
 - [3442: PHPStan Level 6](doctrine#3442) thanks to @morozov
 - [3436: PHPStan Level 5](doctrine#3436) thanks to @morozov
 - [3435: PHPStan Level 4](doctrine#3435) thanks to @morozov
 - [3432: Updated PHPStan to v0.11](doctrine#3432) thanks to @morozov

**Test suite improvements:**

 - [3705: Don't skip a test for sqlite](doctrine#3705) thanks to @Federkun
 - [3689: Updated PHPUnit to 8.4.1](doctrine#3689) thanks to @morozov
 - [3664: Refactor usage of MockBuilder's deprecated setMethods()](doctrine#3664) thanks to @baspeeters
 - [3643: Bumped PHPUnit requrement to ^8.3.3, removed dependency on symfony/phpunit-bridge](doctrine#3643) thanks to @morozov
 - [3609: Reworked the mocks generated by Prophecy using PHPUnit](doctrine#3609) thanks to @morozov
 - [3608: Added a unit test for Doctrine\DBAL\Logging\LoggerChain](doctrine#3608) thanks to @morozov
 - [3600: Updated PHPUnit to 8.2.1](doctrine#3600) thanks to @morozov
 - [3575: Enforced parameter and return value types in test classes](doctrine#3575) thanks to @morozov
 - [3566: Upgraded to PHPUnit 8.1.6 and reworked the remaining driver exception mock](doctrine#3566) thanks to @morozov
 - [3555: Removed the rest of mock classes](doctrine#3555) thanks to @morozov
 - [3546: Reworked driver exception tests](doctrine#3546) thanks to @morozov
 - [3530: Improve ExceptionTest::testConnectionExceptionSqLite](doctrine#3530) thanks to @jwage
 - [3474: Remove more hard-coded mock classes](doctrine#3474) thanks to @morozov
 - [3470: Replaced MockPlatform with the ones generated by PHPUnit](doctrine#3470) thanks to @morozov
 - [3468: Marked some test classes abstract](doctrine#3468) thanks to @morozov
 - [3446: Upgraded PHPUnit to 8.0](doctrine#3446) thanks to @morozov

**Documentation improvements:**

 - [3616: Fix typo in docblock](doctrine#3616) thanks to @rdarcy1
 - [3559: add .github/FUNDING.yml](doctrine#3559) thanks to @jwage
 - [3556: Removed 2.8 from README](doctrine#3556) thanks to @morozov
 - [3514: Expand list of keywords in composer.json](doctrine#3514) thanks to @Majkl578
 - [3504: fix doctrine#3479 (typos in example-code in QueryBuilder)](doctrine#3504) thanks to @DavidBruchmann
 - [3503: Fix the branch alias for master](doctrine#3503) thanks to @stof
 - [3463: fixed a typo in PoolingShardConnection phpdoc](doctrine#3463) thanks to @adapik
 - [3408: Removed unused build files](doctrine#3408) thanks to @morozov
 - [3404: Link to Slack instead of Gitter](doctrine#3404) thanks to @greg0ire
 - [3376: Bump version to 2.10.0-DEV](doctrine#3376) thanks to @morozov

**CI improvements:**

 - [3688: Temporarily disable the usage of PHPUnit 8.4 due to a regression](doctrine#3688) thanks to @morozov
 - [3654: fix Appveyor builds](doctrine#3654) thanks to @mmucklo
 - [3644: Using command line options to configure MySQL 8 instead of mounting a config file](doctrine#3644) thanks to @morozov
 - [3509: Enabled the build against IBM DB2 on PHP 7.3 on Travis CI](doctrine#3509) thanks to @morozov
 - [3528: Reworked SQL Server installer on Travis CI](doctrine#3528) thanks to @morozov
 - [3484: Use latest stable versions of sqlsrv and pdo&doctrine#95;sqlsrv on CI](doctrine#3484) thanks to @morozov
 - [3475: Make PHP 7.3 the primary PHP version on Travis CI](doctrine#3475) thanks to @morozov
 - [3473: Avoid database connection from PHPUnit data providers](doctrine#3473) thanks to @morozov
 - [3469: Replaced Xdebug with PCOV for code coverage](doctrine#3469) thanks to @morozov
 - [3405: Travis CI: PHP 7.3 tests on MySQL 8.0](doctrine#3405) thanks to @BenMorel
 - [3394: Grouped PHPStan and PHP&doctrine#95;CodeSniffer for parallel execution](doctrine#3394) thanks to @morozov
 - [3388: Require PHP 7.2, drop <7.2 in Composer & on CI](doctrine#3388) thanks to @morozov
 - [3122: Introduced a smoke testing phase on Travis to run SQLite tests and CS checks first](doctrine#3122) thanks to @morozov

**Deprecations:**

 - [3708: New deprecations for 2.10](doctrine#3708) thanks to @morozov and @jwage
 - [3598: Deprecated some unused code bits](doctrine#3598) thanks to @morozov
 - [3558: Deprecated Driver::getName()](doctrine#3558) thanks to @morozov
 - [3554: The usage of user-provided PDO instance is deprecated](doctrine#3554) thanks to @morozov
 - [3542: Deprecate SQLSrvStatement::LAST&doctrine#95;INSERT&doctrine#95;ID&doctrine#95;SQL constant.](doctrine#3542) thanks to @jwage
 - [3541: Deprecate the public constants in SQLParserUtils](doctrine#3541) thanks to @jwage

# gpg: Signature made Sun Nov  3 17:56:11 2019
# gpg:                using RSA key 374EADAF543AE995
# gpg: Can't check signature: public key not found

# Conflicts:
#	.travis.yml
#	README.md
#	lib/Doctrine/DBAL/Platforms/SQLAnywherePlatform.php
#	lib/Doctrine/DBAL/Schema/MySqlSchemaManager.php
#	lib/Doctrine/DBAL/Schema/Table.php
#	lib/Doctrine/DBAL/Version.php
#	phpcs.xml.dist
#	tests/Doctrine/Tests/DBAL/Functional/ConnectionTest.php
#	tests/Doctrine/Tests/DBAL/Functional/ExceptionTest.php
#	tests/Doctrine/Tests/DBAL/Functional/MasterSlaveConnectionTest.php
#	tests/Doctrine/Tests/DBAL/Functional/Platform/DefaultExpressionTest.php
#	tests/Doctrine/Tests/DBAL/Functional/ResultCacheTest.php
#	tests/Doctrine/Tests/DBAL/Platforms/SQLAnywherePlatformTest.php
#	tests/Doctrine/Tests/DBAL/Schema/ComparatorTest.php
#	tests/travis/mysql.docker.travis.xml
#	tests/travis/mysqli.docker.travis.xml
@morozov morozov removed the request for review from a team October 23, 2020 17:01
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants